Skip to content

refactor: Improve http error messages#1294

Open
sgtcortez wants to merge 1 commit into
adrienverge:masterfrom
sgtcortez:master
Open

refactor: Improve http error messages#1294
sgtcortez wants to merge 1 commit into
adrienverge:masterfrom
sgtcortez:master

Conversation

@sgtcortez
Copy link
Copy Markdown

Tried to connect to a new vpn today.
However, my user was not allowed to connect.

The logs were not useful, it was only showing Http Status Code.
So, I did this little change, just as logging improvement

image

I also added the status code description after I took this screenshot

@sgtcortez sgtcortez force-pushed the master branch 2 times, most recently from 73a559a to d1bc6e8 Compare August 19, 2025 16:02
Comment thread src/http.h Outdated
return "HTTP status code: 500 - Internal Server Error";
default: return "HTTP status code";
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be possible to print to a string with something like this:

"HTTP status code: %d - %s", code, some_message

But I cannot find an easy solution.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, no easy solution, if we want to create dynamic strings.

we could refactor to use heap segment instead of data segment, but this would imply in refactoring and also, making the callers responsible to free.
we could have a global buffer variable and just update this buffer when this function its called, however, this might cause race condition bugs.

Simple solution and the "correct" for this use case, would be having a big switch statement to handle all the possible status code.
I wrote only I few, cause I am pretty sure that these are the common ones.

Copy link
Copy Markdown
Collaborator

@DimitriPapadopoulos DimitriPapadopoulos Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this part of the code is used by multiple threads, but I don't like the idea of code that is not thread-safe any way. So let's merge this as is. However, could you fix the CI issues first?

You can run tests/lint/run.sh locally.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do it soon.

Thanks @DimitriPapadopoulos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants